Skip to content

[Nexthop] Remove stale ports from portDisplayMap_ on switch state update - #1504

Open
raghavv-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:remove-stale-ports-from-portdisplaymap
Open

[Nexthop] Remove stale ports from portDisplayMap_ on switch state update#1504
raghavv-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:remove-stale-ports-from-portdisplaymap

Conversation

@raghavv-nexthop

@raghavv-nexthop raghavv-nexthop commented Aug 11, 2026

Copy link
Copy Markdown

Pre-submission checklist

  • I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running pip install -r requirements-dev.txt && pre-commit install
  • pre-commit run

Summary

Overview

Remove ports from portDisplayMap_ when they are no longer present in the latest full agent switch-state update. This fixes an LED-state mismatch after breakout reconfiguration, where a previously active port remains in the LED manager’s software state and is incorrectly included when calculating the LED state of a surviving port that shares physical LEDs.

Problem

On Wedge800, the LedServiceTest.testTcvrLos test fails LED color equality checks and produces error messages that look like the following:

/src/fboss/led_service/hw_test/LedServiceTest.cpp:180: Failure
Expected equality of these values:
  state.ledColor().value()
    Which is: 4-byte object <0A-00 00-00>
  color
    Which is: 4-byte object <06-00 00-00>
LED state in IO should be BLUE for port eth1/3/5 but current color is YELLOW for test 11
/src/fboss/led_service/hw_test/LedServiceTest.cpp:672: Failure
Expected equality of these values:
  ledState.ledColor().value()
    Which is: 4-byte object <0A-00 00-00>
  led::LedColor::BLUE
    Which is: 4-byte object <06-00 00-00>
LED 8 should be BLUE for port eth1/3/5 test 12

Cause

LedManager::updateLedStatus() receives a complete view of the active agent ports through the FSDB switch-state subscriber. It updated or inserted entries in portDisplayMap_, but did not remove entries for ports that disappeared from the new switch state.

After a breakout change, this left a stale port in portDisplayMap_. BspLedManager::getCommonLedSwPorts() considers all switch ports supported by the transceiver and identifies ports that share LED IDs. As a result, the stale port continued to participate in the shared-LED aggregation for its breakout sibling.

On Wedge800, this can produce the following mismatch:

  • The active 400G port has LOS on all of its lanes.
  • The stale sibling is counted as a second shared port.
  • The aggregate calculation sees one LOS port out of two and returns BLUE+SLOW.
  • The per-LED LOS path drives the physical LEDs YELLOW.

The cached LED state and the hardware LED state then disagree, causing LedServiceTest.testTcvrLos to fail.

The stale-entry bug is generic, but Wedge800 makes the resulting inconsistency observable because its relevant transceiver mapping has four LEDs across eight lanes—two lanes per LED.

A 400G breakout port therefore spans multiple LEDs and uses the per-LED LOS handling path. On platforms where a 400G port maps to a single LED, the aggregate state is written directly to that LED, so the stale-port condition does not produce the same cache-versus-hardware mismatch.

Solution

After processing the latest switch-state entries, erase every portDisplayMap_ entry whose port ID is absent from the new switch state.

The existing LED update flow then recalculates the state using only active ports. For the failing breakout case, the aggregate calculation correctly sees one active port with LOS and returns YELLOW+SLOW, matching the LED state written by the existing hardware path.

Follow-up Defects

While the change in this PR alone results in the LedServiceTest.testTcvrLos test passing, there are two other defects that debugging this failure exposed:

1. Per-LED LOS handling treats missing lane data as LOS

setLedBasedOnLOS() initializes allLanesLos to true and only changes it when a lane explicitly reports rxLos=false. If an LED lane is missing from portLosMap_, the function logs the missing entry but leaves allLanesLos set to true, causing that LED to be driven yellow instead of treating the data as unknown or incomplete.

2. Per-lane LOS changes may not repaint the LEDs

triggerLedUpdate() calls setLedState() only when the aggregate LedState changes. On platforms where one port controls multiple LEDs, the aggregate state can remain BLUE+SLOW while the per-LED LOS pattern changes—for example, from YELLOW/YELLOW to YELLOW/BLUE.

In that case, the cached aggregate state is unchanged, so setLedState() is skipped and the hardware LEDs retain the previous per-LED pattern. A complete fix should trigger a hardware update when the per-lane LED representation changes, not only when the aggregate color or blink state changes.

Test Plan

To verify the change, I ran the full LED service test suite on two platforms (Wedge800bact and Montblanc) using this command:

./led_service_hw_test --led-config /tmp/led.conf 

Result on Wedge800bact:

[----------] 5 tests from LedServiceTest (1116 ms total)

[----------] Global test environment tear-down
[==========] 5 tests from 1 test suite ran. (1116 ms total)
[  PASSED  ] 5 tests.
[user@dut dir]#

Result on Montblanc:

[----------] 5 tests from LedServiceTest (4129 ms total)

[----------] Global test environment tear-down
[==========] 5 tests from 1 test suite ran. (4129 ms total)
[  PASSED  ] 5 tests.
[user@dut dir]#

Signed-off-by: raghavv-nexthop <raghavv@nexthop.ai>
@meta-cla meta-cla Bot added the CLA Signed label Aug 11, 2026
@raghavv-nexthop
raghavv-nexthop marked this pull request as ready for review August 11, 2026 22:07
@raghavv-nexthop
raghavv-nexthop requested a review from a team as a code owner August 11, 2026 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant